feat: chat transcript mr2a html export - #10076
Conversation
Establish a shared transcript model and document-mode projection so Web Shell, VS Code, and HTML export can consume the same stable conversation semantics without changing interactive rendering. - Preserve daemon and ACP segment identity across replay and normalization - Add export-safe previews and a versioned transcript document builder - Add document-mode Web Shell rendering with bounded Mermaid processing - Lock direct-daemon and ACP behavior with contract fixtures - Cover render and export equivalence in integration tests
Move transcript prevalidation onto real product paths while preserving the legacy VS Code timeline as the default fallback. - Add version-bound document HTML with strict CSP and schema checks - Add opt-in ACP projection with stable source identity - Reuse product export code across hosts and the integration runner - Remove test-only production probes and consolidate shared validation Note: Browser, host-action, VSIX, and packaging gates remain pending.
Keep the npm lockfile aligned with the VS Code and HTML export package manifests added by the transcript consumer migration. - Lock the VS Code Web Shell workspace dependency - Lock Web Templates SDK, Web Shell, and React build dependencies
…plit-20260818 # Conflicts: # packages/acp-bridge/src/transcript-replay.ts # packages/sdk-typescript/scripts/build.js # packages/sdk-typescript/src/daemon/ui/transcript.ts # packages/web-shell/client/components/MessageList.tsx # packages/web-shell/client/components/messages/GoalStatusMessage.tsx
Resolve review findings across document export and the VS Code ACP timeline while preserving default interactive and readonly semantics. - Preserve stable text and non-text identity across live and replay - Harden export projection, budgets, URL handling, CSP, and nonces - Make document rendering complete, inert, and browser-validated - Keep VS Code transcript scope, theme, copy, and flags reactive - Restore fixture, schema, hash, and compatibility contract locks Note: The overall gate remains failed pending VSIX, host-action, and packaged-artifact evidence.
…eb-shell-adapter-before-split-20260818 # Conflicts: # packages/web-shell/client/components/MessageList.tsx # packages/web-shell/client/components/messages/AssistantMessage.tsx # packages/web-shell/client/components/messages/ToolGroup.tsx
…eb-shell-adapter-before-split-20260818
Close the latest review findings across transcript identity, VS Code rehydration, and the HTML export security boundary. - Terminate discrete ACP segments and bind automatic turn provenance - Reset VS Code transcript scopes across replay and reconnect lifecycles - Align Markdown sanitization and envelope budgets with document rendering - Run browser gates with Chromium and complete third-party notices
Reduce the shared transcript review surface while keeping the VS Code and HTML Export consumers and their security gates intact. - Merge VS Code feature state into the ACP transcript hook - Collect source identity once and strip it at the compatibility boundary - Remove test-only export adapters, gate reports, and duplicate helpers - Keep shell output separated when stable producer segments change - Mark third-party notices as generated review content
Close the latest review findings around document resource safety and recorded-user replay consistency. - Redact local home paths structurally without corrupting remote URLs - Sanitize nested Markdown images inside otherwise safe links - Advance merged segment provenance without duplicating separators - Keep recorded-user stable IDs anchored to durable record identity
…plit-20260818 # Conflicts: # packages/cli/src/acp-integration/session/Session.ts # packages/web-shell/client/components/MessageList.tsx # packages/web-shell/client/components/WebShellTranscript.tsx # packages/web-shell/client/components/messages/ToolGroup.test.tsx # packages/web-shell/client/components/messages/ToolGroup.tsx
Keep MR2A focused on the document export pipeline while preserving the legacy VS Code timeline. - Retain VS Code /export html as a real document consumer - Move live transcript adapters, identity wiring, and feature flag to MR2B - Remove the direct Web Shell dependency and restore baseline notices - Keep candidate identity evidence in integration-only helpers - Document the MR1, MR2A, and MR2B delivery boundaries
# Conflicts: # packages/web-shell/client/components/messages/ToolGroup.tsx
# Conflicts: # packages/web-shell/client/components/WebShellTranscript.tsx # packages/web-shell/client/components/messages/PlanMessage.test.tsx # packages/web-shell/client/components/messages/TodoView.tsx # packages/web-shell/client/components/messages/ToolGroup.tsx
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 19 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks for the PR! This is MR2A of the shared chat transcript design — a planned milestone, and it reads that way. Gate notes below. Template ✓ — all required sections are present. One small nit: the English half omits the Problem: this is a planned feature milestone, not theoretical hardening. It implements the merged design doc Direction: aligned. The design doc — merged by a maintainer in #9388 on 2026-08-19 — prescribes exactly this split: export builder + document mode + HTML wiring + browser/security gates now, VS Code live timeline migration deferred to MR2B with the gate intentionally left at Size: large. 11,210 changed lines across 96 files and 7 packages (cli, core, sdk-typescript, acp-bridge, web-shell, web-templates, vscode-ide-companion) — of which ~6,101 are production logic (68 files), 5,000 are tests (27 files), and 109 are schema. For a Approach: the scope matches the design doc's consumer-ordered plan — the centerpiece is the new closed Risk: no matches on the revert-correlated high-risk paths. Elevated attention goes to two places instead: the shared web-shell components (also serving the live interactive shell) and the security boundary itself — CI evidence will matter, and the ubuntu lane is currently red (details in the review comment). Moving on to code review. 🔍 中文说明感谢贡献!这是共享 chat transcript 设计的 MR2A——一个有计划的里程碑,整体也是如此呈现的。 模板 ✓ — 所有必需章节齐全。小问题:英文部分缺少中文部分包含的 问题:这是有计划的特性里程碑,不是理论性加固。它实现了已合并的设计文档 方向:对齐。设计文档(由维护者在 #9388 于 2026-08-19 合并)恰好规定了这一拆分:先做 export builder + document mode + HTML wiring + 浏览器/安全门禁,VS Code live timeline 迁移推迟到 MR2B,门禁有意保持 规模:大。96 个文件、7 个包共 11,210 行改动,其中约 6,101 行为生产逻辑(68 个文件),5,000 行为测试(27 个文件),109 行为 schema。 方案:范围符合设计文档按消费者排序的计划——核心是新的封闭式 风险:未命中与 revert 相关的高风险路径。需要重点关注两处:共享 web-shell 组件(同时服务于 live 交互界面)和安全边界本身——CI 证据很重要,而 ubuntu 通道当前是红的(详见审查评论)。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI read the security boundary in detail — it is the heart of this PR, and it holds up: per-kind What does need attention:
sequenceDiagram
participant P1 as ChatRecord records
participant P2 as Record export policy
participant P3 as SDK canonical projector
participant P4 as Document projector
participant P5 as Gates
participant P6 as CSP template
participant P7 as Browser gate
P1->>P2 keep visible record kinds only
P2->>P3 project to transcript blocks
P3->>P4 typed allowlist, opaque IDs, redaction, budgets
P4->>P5 schema plus semantic re-validation, fail closed
P5->>P6 inject escaped JSON under nonce CSP
P6->>P7 zero network, canary, search copy print probes
Files changed (30 of 96 shown)
Test evidence (this PR's own CI — unattended run, no PR code executed here)The PR's central lane is red on the reviewed commit. I verified this test exists at line 718 of
Sandboxed verification would settle the behavioural claims once CI is green: 中文说明代码审查:安全边界是本 PR 的核心,审查结论是站得住的——逐 kind 的 需要关注的:
测试证据(引用本 PR 自身 CI,本次为无人值守运行,未执行任何 PR 代码):核心通道 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 2/5 — the security boundary is genuinely well built, but the PR's own CI fails a pre-existing test that pins the exact "live behavior preserved" claim it makes, and a change of this size needs a maintainer in the loop regardless. Stepping back: my independent take on this problem would have been almost exactly what this PR does — a closed, versioned document type with per-kind allowlists, a one-way projector that never serializes raw payloads, reuse of the existing canonical projection, a nonce-CSP template with zero remote surface, and all hosts converged on one renderer. The implementation matches or exceeds that, and the defensive layers (forbidden-field scan, second full re-validation, But two things keep this from a clean pass:
Verdict: request changes on the red CI (fix or prove flake), and defer the rest to @yiliang114 — you merged the prevalidation stage (#9388) this builds on; the questions worth your eyes are the SDK preview-semantics scope and whether MR2A should land whole or split at the schema/projector seam. @water-in-stone strong work on the boundary itself; the blocker is narrow and concrete. 中文说明置信度:2/5 — 安全边界构建得确实很好,但 PR 自身的 CI 在一个既有测试上失败,而该测试固定的正是本 PR 所声称的"保持 live 行为";且无论如何在这样规模的改动下都需要维护者介入。 独立来看,我对这个问题的方案与本 PR 几乎一致:封闭的、版本化的文档类型加逐 kind 白名单、永不序列化 raw payload 的单向投影器、复用现有 canonical 投影、nonce-CSP 模板且零远程资源、所有宿主收敛到同一渲染器。实现达到甚至超过了这一方案,防御层次(禁止字段扫描、第二轮完整复验、 但有两点使其无法干净通过:
结论:就红的 CI 请求修改(修复或证明是 flake),其余交由 @yiliang114 定夺——您合并了本 PR 所基于的 prevalidation 阶段(#9388);值得您关注的是 SDK 预览语义的范围,以及 MR2A 应整体合入还是在 schema/投影器接缝处拆分。@water-in-stone 边界本身做得很好;阻塞点是窄而具体的。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Requesting changes on one concrete blocker: the ubuntu CI lane is red on this head — "MessageList — compact mode > keeps a folded single-agent summary separate from an approving agent" fails in components/MessageList.dom.test.tsx (expected the msg-summary-t1 node to render, got null). That test is unchanged from main, while the grouping logic it pins is exactly what this PR extracted and retyped. Please fix it or demonstrate it is a flake with a re-run.
Everything else is in my review comments above — the security boundary reads well; the open items for the maintainer are the SDK preview-semantics scope and the MR2A size/split question. See the stage comments for details. 🙏
|
Follow-up on the change-request review, Stage 2 triage, and Stage 3 triage on current PR head All 33 inline review threads now have an author reply and are resolved:
Validation on this head:
I left the historical |
|
Final main synchronization update: PR head is now I reran the relevant validation after this merge: repository build and typecheck passed; CLI export 54/54, SDK transcript/UI 391/391, Web Shell relevant tests 686/686 (including the compact-mode regression), VS Code export 9/9, transcript contract 3/3, Chromium document gate 4/4, and the isolated no-access-key integration gate 151/151 across 13 files all passed. All 33 inline review threads remain replied to and resolved. The historical |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): chunk 23: none — no checks were cut short.; chunk 15: executing packages/cli/src/ui/utils/export/export-transcript-document.test.ts to confirm green at HEAD — the review worktree has no node_modules /dist and a ….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
integration-tests/chat-transcript-contract.test.ts:225 — [review] Schema-closure lock misses the toolResultPreview union (mutation stays green)integration-tests/chat-transcript-contract.test.ts:91 — [review] Fixture-tree lock is fail-open for symlink entriesdocs/design/web-shell/chat-transcript-contract-prevalidation.md:690 — [review] Six bare MR2 references orphan gate obligations after the MR2A/MR2B splitintegration-tests/chat-transcript-document.test.ts:6 — [review] Browser gate imports playwright as a phantom dependency (webui hoist only)packages/web-shell/client/adapters/parallelAgentGrouping.ts:30 — [review] Summary-row exclusion hardcodes summary- instead of the shared summaryRunId predicatepackages/acp-bridge/src/transcript-replay.ts:1262 — [review] Unreachable shell_output/tool_output lane branch in transcriptSegmentLanepackages/web-templates/src/export-html/build.mjs:142 — [review] Post-transform <script guard is dead code; misleading safety modelpackages/web-shell/client/components/MessageList.tsx:2093 — [review] data-source-block-ids written on every row but read nowhere; includeSourceIdentity hard-wired for itpackages/web-shell/client/adapters/transcriptToMessages.ts:480 — [review] sourceBlockIds missing on three block-backed pushes (mid_turn/vision_bridge)
中文说明
仅完成部分审查,审查缺口已披露。
未探索到全部深度(达到工具调用预算):chunk 23:none — no checks were cut short.;chunk 15:executing packages/cli/src/ui/utils/export/export-transcript-document.test.ts to confirm green at HEAD — the review worktree has no node_modules /dist and a …。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 9 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
# Conflicts: # packages/sdk-typescript/scripts/build.js
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- dead post-rewrite <script guard in packages/web-templates/src/export-html/build.mjs:142 — already reported on the PR (round-2 deferred list, review 5041220005)
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": exhaustive enumeration of the document renderer's markdown surface via the ExportTranscriptBlockV1→WebShellTranscript adapter — specifically whether generic pr…; chunk 39: executing ToolGroup.test.tsx under vitest to confirm the new tests are green — the review worktree has no node_modules and a monorepo install + build exceed…; chunk 36: executing Markdown.mermaid.test.ts and Markdown.test.ts at HEAD (no node_modules in the review worktree; install+build exceeds chunk budget); chunk 22: none — the only unperformed check (live vitest run of the new test file) was impossible for environment reasons (no installed dependencies in the review workt…; chunk 37: none — I could not install/inspect mermaid's bundled .d.ts locally (no node_modules in the worktree), but resolved this via the lockfile version and mermaid….
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/components/messages/AssistantMessage.tsx:46 — [review] showFooter document-mode guard has no witness testpackages/cli/src/ui/utils/export/formatters/html.ts:147 — [review] CreateExportTranscriptDocumentOptions.title is a dead switch.github/workflows/ci.yml:846 — [review] transcript-document browser gate wiring has no contract pinintegration-tests/chat-transcript-document.test.ts:551 — [review] renderer-side budget guards (schemaVersion/blocks/envelope) have no failing testpackages/cli/src/ui/utils/export/export-transcript-document.ts:1193 — [review] APPROVED_PERMISSION_TOKENS omits proceed_always-family approval outcomespackages/web-shell/client/components/MessageList.module.css:248 — [review] dead document-mode CSS for .sessionTimelineViewport contradicts hideSessionTimelinepackages/web-shell/client/components/MessageTimestamp.tsx:44 — [review] MessageTimestamp document-mode guard has no witness testpackages/web-shell/client/components/messages/UserShellMessage.module.css:16 — [review] document mode de-clips the shell box but the command header keeps its ellipsis
Convergence: round 3 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 12 (12 new). Findings keep coming back to the same files: packages/cli/src/ui/utils/export/markdown-document-policy.ts (findings in rounds 1, 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)":exhaustive enumeration of the document renderer's markdown surface via the ExportTranscriptBlockV1→WebShellTranscript adapter — specifically whether generic pr…;chunk 39:executing ToolGroup.test.tsx under vitest to confirm the new tests are green — the review worktree has no node_modules and a monorepo install + build exceed…;chunk 36:executing Markdown.mermaid.test.ts and Markdown.test.ts at HEAD (no node_modules in the review worktree; install+build exceeds chunk budget);chunk 22:none — the only unperformed check (live vitest run of the new test file) was impossible for environment reasons (no installed dependencies in the review workt…;chunk 37:none — I could not install/inspect mermaid's bundled .d.ts locally (no node_modules in the worktree), but resolved this via the lockfile version and mermaid…。
未审查:反向审计——评审时间预算不足,未能开始第 2 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 3 条行内评论,其中 2 条是首次提出;上一轮发布了 12 条(其中 12 条首次提出)。发现反复回到同一批文件:packages/cli/src/ui/utils/export/markdown-document-policy.ts(第 1、2 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.2)
yiliang114
left a comment
There was a problem hiding this comment.
Checked this head against the round-2 findings — the fixes look real: block.files now emits a file_attachment_excluded diagnostic and marks content loss, label() re-runs safeLabel after home-path redaction, link destinations are wrapped in angle brackets, wasCancelled propagates through mergeToolCall, delete-only diffs render, both Playwright revisions install, and the complexity veto is fence-aware with emphasis-delimiter counting. Two inline items still block merge.
One more blocker that doesn't map to a single line: the web-shell E2E Smoke lane is red on this head, and the signature is not a flake — [data-web-shell-root] never renders on /session/:id, so multiple specs (interaction-chain-smoke, collapsed-groups-persist, ...) fail through all 3 retries. The new ExportTranscriptDocument browser gate passes in the same run, which points at the main Web Shell mount path rather than the new document renderer. Worth reproducing locally and root-causing before the next push — this PR touches App.tsx / MessageList / useMessages / the adapters, so it's the first suspect.
Non-blocking: the 9 deferred items disclosed in the review summary (schema-closure lock missing the toolResultPreview union, symlink fail-open fixture lock, playwright phantom dependency, unread data-source-block-ids, etc.) are worth a follow-up issue at merge time so they don't get dropped.
| const defaultRootDir = path.resolve(__dirname, '..'); | ||
| const TEST_FILE_RE = /\.(test|spec)\.(d\.)?[mc]?[jt]s(\.map)?$/; | ||
| const DEFAULT_MAX_NPM_PACKAGE_UNPACKED_BYTES = 96 * 1024 * 1024; | ||
| const DEFAULT_MAX_NPM_PACKAGE_UNPACKED_BYTES = 128 * 1024 * 1024; |
There was a problem hiding this comment.
[Decision needed] R2-7 is resolved here by raising the budget from 96 to 128 MiB (measured package is ~117.8 MB). That unblocks the release lane, but it makes the ~20 MB inlined document template permanent in the npm package. The reviewer's other option was slimming the template (lazy-load or compress). Either is defensible, but a maintainer should pick it explicitly in this PR rather than letting the bump be the default.
There was a problem hiding this comment.
Not resolved yet on PR head 64cc57a59c.
The current branch keeps the explicit 128 MiB budget, and the real packaging flow passes at ~117.8 MB. The alternative—compressing or lazy-loading the ~20 MB renderer—changes the packaging/runtime design. As requested, I am leaving this thread open until a maintainer explicitly chooses which direction this PR should take.
Harden the document export boundary after the latest review found mixed-path and Markdown parser/scanner gaps. - Normalize dot segments and mixed separators before home redaction - Align Markdown preflight with fences, HTML comments, and delimiters - Remove the internal review report from the tracked repository
|
Follow-up on the round-3 review and maintainer review at current PR head
Validation for the published fixes: CLI Export/Markdown 53/53, repository build and typecheck passed, transcript contract + Chromium document gate 7/7, and focused lint/format/diff checks passed. The historical |
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
# Conflicts: # packages/web-shell/client/adapters/transcriptToMessages.ts # packages/web-shell/client/components/messages/UserMessage.test.tsx
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- APPROVED_PERMISSION_TOKENS omits proceed_always-family approval outcomes — already reported (round-3 deferred list, review 5051592643)
- Unreachable shell_output/tool_output lane branch in transcriptSegmentLane incl. parentToolCallId ''-vs-absent facet — already reported (round-2 deferred list, review 5041220005)
- sourceBlockIds missing on the three block-backed pushes (mid_turn/vision_bridge) — already reported (round-2 deferred list, review 5041220005)
- data-source-block-ids written on every row but read nowhere; includeSourceIdentity hard-wired for it — already reported (round-3 deferred list, review 5051592643)
- Six bare MR2 references orphan gate obligations after the MR2A/MR2B split — already reported (round-2 deferred list, review 5041220005)
- showFooter document-mode guard has no witness test (AssistantMessage.tsx:46) — already reported (round-3 deferred list, review 5051592643)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 10: none — no checks were cut short.; "agent reverse-audit (round 1)": executing packages/web-shell transcriptToMessages.test.ts under vitest (needs workspace dist builds) — all 13 new tests verified by static trace only, not by a ….
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/components/artifacts/turnOutputSelectors.test.ts:794 — [review] write_file content/newText precedence unpinned — key-swap mutation survives the suiteintegration-tests/concurrent-runner/export-html-from-chatrecord-jsonl.js:12 — [review] nothing anywhere exercises the rewritten concurrent-runner exporterintegration-tests/concurrent-runner/export-html-from-chatrecord-jsonl.js:12 — [review] full product pipeline under runner.py's fixed 30s timeout — in-budget session measured at 28.21s of the kill pointpackages/cli/src/ui/utils/export/export-transcript-document.test.ts:855 — [review] OpaqueDocumentIds referential integrity unpinned — fresh-id-per-occurrence mutation keeps the suite greenpackages/cli/src/ui/utils/export/export-transcript-document.ts:411 — [review] 5 of 7 VISIBLE_SYSTEM_RECORD_SUBTYPES entries are unreachablepackages/web-shell/client/components/artifacts/turnOutputSelectors.ts:389 — [review] safe-projection fallback covers only write_file — safe-projected edits render diff cards with no diffs
Convergence: round 4 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 3 (2 new). Findings keep coming back to the same files: packages/cli/src/ui/utils/export/markdown-document-policy.ts (findings in round 2; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 6 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):chunk 10:none — no checks were cut short.;"agent reverse-audit (round 1)":executing packages/web-shell transcriptToMessages.test.ts under vitest (needs workspace dist builds) — all 13 new tests verified by static trace only, not by a …。
未审查:反向审计——评审时间预算不足,未能开始第 2 轮。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/ci.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 6 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 3 条行内评论,其中 2 条是首次提出;上一轮发布了 3 条(其中 2 条首次提出)。发现反复回到同一批文件:packages/cli/src/ui/utils/export/markdown-document-policy.ts(第 2 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.3)
Close three Critical review findings in the document export and safe transcript projection paths. - Share percent-token decoding between home-path redaction and checks - Reject parser/scanner divergence before Markdown parsing can stall - Preserve todo entries carried by the document-safe projection - Add regression witnesses for each reported failure mode
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
yiliang114
left a comment
There was a problem hiding this comment.
Risk-directed review of 44fa755e1f: I traced the current HTML-export path from CLI/serve/VS Code through the safe transcript projection, document-mode Web Shell adapter, template injection/CSP, and the current check results. I found one new security-boundary blocker below.
The earlier release-lane failure is mechanically fixed by the 128 MiB package cap, but that choice permanently adds roughly 20 MiB to the shipped npm package (measured package ~117.8 MB). Please treat that as an explicit maintainer product decision, not as a neutral test-only adjustment; my recommendation is to slim/compress the embedded renderer if that can be kept within this PR.
CI has not produced a failing product assertion on this head: the main Qwen Code CI jobs were cancelled mid-run, and the automatic review job failed in review orchestration while attempting a 48-agent fan-out. Those runs therefore do not establish a green full gate.
|
|
||
| function decodePercentToken(value: string): string { | ||
| let decoded = value; | ||
| for (let pass = 0; pass < 3; pass += 1) { |
There was a problem hiding this comment.
[Critical] Four percent-encoding layers still bypass the home-path boundary. decodePercentToken stops after exactly three passes, and both the redactor and the final residual validator call this same bounded decoder. For attacker-controlled transcript text, a four-times encoded path such as %2525252Fhome%2525252Falice%2525252Fprivate.txt is reduced after three passes only to %2Fhome%2Falice%2Fprivate.txt; no raw-home pattern matches, so the document is accepted and ships that token unchanged. One further ordinary URI decode by a recipient reveals /home/alice/private.txt. This is reachable through user/assistant/tool-result text and violates the PR's stated no-home-path export boundary.
Please make the bounded case fail closed: if the pass limit is reached while a token still contains recursively decodable escapes, redact or reject that token (or use a linear, token-length-bounded normalization that reaches a fixed point). Add a regression with a four-layer encoded Unix and Windows home path and assert that neither the encoded token nor the decoded username-bearing path survives.
HTML export UI regression reportTested scope: PR #10076, current head Method: I generated the legacy and WebShell HTML from the same representative Case matrix
Before / After — same representative fixture
Responsive width — same fixture at 480 px
Recovery and fail-closed states
Remaining blockers / decisions
Conclusion: the WebShell document renderer is a functional upgrade for transcript content and its desktop/narrow standalone rendering is sound, but the PR is not ready to merge yet because of the unresolved security Critical and explicit legacy-parity losses above. |
|
Follow-up on the product-direction items in the UI regression report: P02-P05 should no longer be treated as open decisions. The migration should preserve existing useful export behavior unless there is a concrete incompatibility. Before merge, the WebShell document export should retain the legacy metadata summary (repository/branch/model/channel/tokens/session and file-operation information), Context Usage, theme behavior, and screen-level expand/collapse controls. Print CSS can still force everything expanded. The boundary is also explicit: this is a static, read-only session-history export. It should not render a composer, send/attachment/voice controls, model or permission selectors, connect to the daemon, or expose actions that continue the conversation or execute tools. New document rendering capabilities such as Mermaid, math, typed tool output, diff, plan, ask-user history, and nested-agent history are welcome, but they should be additive rather than replacing legacy information. Please keep these adaptations behind document mode so existing interactive/readonly WebShell consumers are unchanged, and render restored metadata only after the existing export sanitization policy has been applied. I consider the missing legacy behaviors in scope for this migration, separately from the unresolved security and CI blockers. |






What this PR does
This PR delivers MR2A of the shared chat transcript design: a production HTML Export pipeline without migrating the VS Code live timeline.
ExportTranscriptDocumentV1with per-kind allowlists, opaque document IDs, typed tool previews/results, completeness diagnostics, and explicit resource budgets./export html, and the integration runner through the same version-bound product renderer while preserving the legacy HTML call when original records are unavailable.write_fileTurn Output diffs; document projection never falls back to raw payloads.MessageList. MR2A adds no shared renderer dependency, transcript feature flag, live update forwarding, production adapter, or host-action seam; direct-daemon and ACP identity remain candidate evidence only, withselectedVscodePath: nullandoverall: "fail"until MR2B.Why it's needed
The existing hosts have separate transcript and export paths, which risks duplicated replay/rendering behavior and inconsistent portable output. HTML export also needs a one-way security boundary so raw tool payloads, credentials, absolute user paths, session identity, unsafe resources, and unbounded rendering work cannot enter a shareable document. MR2A establishes that bounded document path independently from the higher-risk VS Code live timeline migration, allowing the two changes to be reviewed, rolled out, and reverted separately.
Reviewer Test Plan
How to verify
npm run buildandnpm run typecheck; expect the SDK, CLI, Web Shell library, embedded document renderer, and VS Code companion export consumer to build and type-check while VS Code continues to use its legacy live timeline.npx vitest run --root ./integration-tests ./chat-transcript-contract.test.ts --retry=0; expect fixture hashes, the production schema, raw-free document semantics, and both candidate identity projections to pass whileselectedVscodePathremainsnulland the overall gate remainsfail.write_fileTurn Output diffs, and VS Code/export htmlto pass original records into the shared document pipeline./export html, and the integration runner; expect the same version-bound, self-contained renderer and a safe read-only failure for incompatible schema or renderer versions.NOTICES.txtdelta in MR2A because the live Web Shell timeline dependency is deferred to MR2B.Tested on
Environment (optional)
Risk & Scope
MessageListremains authoritative. Interactive/readonly raw semantics remain the compatibility baseline, document-only safe projection does not flow back into live sessions, and the migration gate intentionally remainsoverall: "fail"withselectedVscodePath: null.Linked Issues
Linked #9387
中文说明
What this PR does
本 PR 交付共享聊天 transcript 设计的 MR2A:落地生产级 HTML Export pipeline,但不迁移 VS Code live timeline。
ExportTranscriptDocumentV1,包含逐 kind allowlist、文档内 opaque ID、typed tool preview/result、完整性 diagnostic 和显式资源预算。/export html和 integration runner 复用同一个版本绑定的产品 renderer;无法取得原始 records 时仍保留 legacy HTML 调用兼容性。write_fileTurn Output diff;document projection 永不回退 raw payload。MessageList。MR2A 不新增 shared renderer 依赖、transcript feature flag、live update 转发、生产 adapter 或 host-action seam;direct-daemon 与 ACP identity 仅作为候选证据保留,在 MR2B 前维持selectedVscodePath: null和overall: "fail"。Why it's needed
现有宿主拥有不同的 transcript 与 export 路径,存在重复 replay/rendering 行为和可移植输出不一致的风险。HTML export 还需要一个单向安全边界,防止 raw tool payload、credential、绝对用户路径、session identity、危险资源和无界渲染工作进入可分享文档。MR2A 将该有界 document path 与风险更高的 VS Code live timeline 迁移分离,使两者可以独立评审、发布和回滚。
Reviewer Test Plan
How to verify
npm run build和npm run typecheck;预期 SDK、CLI、Web Shell library、内嵌 document renderer 和 VS Code companion export consumer 均可构建并通过类型检查,同时 VS Code 继续使用 legacy live timeline。npx vitest run --root ./integration-tests ./chat-transcript-contract.test.ts --retry=0;预期 fixture hashes、生产 schema、raw-free document semantics 和两条 candidate identity projection 均通过,同时selectedVscodePath保持null、overall gate 保持fail。write_fileTurn Output diff 保持完整,并且 VS Code/export html将原始 records 传入共享 document pipeline。/export html和 integration runner 生成 HTML;预期使用同一个版本绑定、自包含的 renderer,并在 schema 或 renderer version 不兼容时安全地只读失败。NOTICES.txtdiff,因为 live Web Shell timeline 依赖已延后到 MR2B。Evidence (Before & After)
未提供 screenshot 或 recording。输入文档记录了可重复 fixtures、SHA-256 锁、capability matrix、聚焦的 SDK/Core/CLI/Web Shell/VS Code export 覆盖、direct-daemon/ACP candidate checks、产品 HTML build assertions,以及覆盖最大文档、主动 CSP enforcement、零意外网络请求、搜索、复制、打印、远程资源降级和时间戳归一化的 Chromium browser gate。
Tested on
Environment (optional)
N/A — 输入文档未注明 runtime environment 或 sandbox。
Risk & Scope
MessageList继续作为权威路径。interactive/readonly raw 语义仍是兼容性基线,document-only safe projection 不会回流 live session,并且 migration gate 有意维持overall: "fail"和selectedVscodePath: null。Linked Issues
#9387
N/A